-
-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: Help older Python parse new Git dates #3721
Conversation
> Current Git (2.45.1) produces strict ISO dates with Z for any time with +00:00 time zone offest. Python prior to 3.11 does not understand these dates. This trunkes the Z in case of ValueError which is what old Python throws. The +00:00 offset is placed in the input string for fromisoformat instead.
At least one of the GCC runs here is using the new image, 20240516.1, and it runs, so I'm assuming the change helped.
Anyway, not my favorite change. This should be removed again when our min Python version is 3.11. |
What if we go another way, and ask Git to output the specific format we want, that would work for any locale settings? I know that for multiple git commands, I already set the format I wanted, it wasn't git log, but another less common command and the usage was similar |
That's what we tried to do with requesting strict ISO. With other things I would perhaps say yes, specifying custom format is reasonable, but going into the whole date formatting, I don't know... What will we do when we drop Python 3.10 support? Go back to the code which is now on main or keep maintaining our own date format just for this piece of code? I say, let's work towards using ISO and this PR does that. I think it follows the trend in Python and Git: Python supported subset of ISO for parsing, now they support the whole thing. Git had some ISO-like format they called ISO and then then introduced strict ISO. Now they made change they believe is closer to the standard (I'm not convinced about that, but that's not the point here). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing the way that this fix is implemented, and verifying that no other time zones end with Z (from lists of time zones wiki pages), I think this fix is reasonable. It tries the happy path, then tries one fix for a common failure, and then fails.
Is this a candidate for backporting? (On what branches the git dates were used?) |
Now merged, multiple builds with Ubuntu 22.04 image 20240516.1 are successful, including ones using Python 3.8 and 3.10. |
No, we don't plan further releases in the current branches. The only planned release in the Milestones is 8.4.0. |
Current Git (2.45.1) produces strict ISO dates with Z for any time with +00:00 time zone offest. Python prior to 3.11 does not understand these dates. This trunkes the Z in case of ValueError which is what old Python throws. The +00:00 offset is placed in the input string for fromisoformat instead.